backend/app/services/health_probes.py (1)

211-220: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a uniqueness assertion to the __main__ self-check.

The __main__ block validates modalities and config building but doesn't check for duplicate probes. Adding assert len(_PROBES) == len(set(_PROBES)) would have caught the duplicate at lines 40-41.

♻️ Proposed addition

 if __name__ == "__main__":
     assert _PROBES, "probe list must not be empty"
+    assert len(_PROBES) == len(set(_PROBES)), "duplicate probe entries detected"
     modalities = {p.modality for p in _PROBES}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/services/health_probes.py` around lines 211 - 220, Add a
duplicate-probe validation to the __main__ self-check in health_probes.py: after
asserting _PROBES is nonempty, assert len(_PROBES) == len(set(_PROBES)) before
validating modalities and config construction.

backend/app/tests/services/test_health_probes.py (1)

131-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Replace the obscure generator-throw lambda with a named function for consistency.

lambda **_k: (_ for _ in ()).throw(RuntimeError("bad")) is unreadable. The ValueError test above (line 117) uses a clear named function _boom — follow the same pattern here.

♻️ Proposed fix

-        monkeypatch.setattr(
-            health_probes,
-            "get_llm_provider",
-            lambda **_k: (_ for _ in ()).throw(RuntimeError("bad")),
-        )
+        def _boom(**_kwargs):
+            raise RuntimeError("bad")
+
+        monkeypatch.setattr(health_probes, "get_llm_provider", _boom)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/tests/services/test_health_probes.py` around lines 131 - 138,
Replace the generator-throw lambda in
test_get_llm_provider_runtime_error_marks_client_init_failed with a local named
function, such as _boom, that accepts arbitrary keyword arguments and raises
RuntimeError("bad"), then monkeypatch get_llm_provider with that function,
matching the pattern used by the ValueError test.

backend/app/celery/tasks/job_execution.py (1)

411-423: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider using dict[str, Any] instead of bare dict for return types.

The upstream run_probes returns dict[str, Any]. Using -> dict[str, Any] on both run_health_probes and the nested _do closure would match that contract and improve type precision.

♻️ Proposed type hint refinement

-def run_health_probes(self, trace_id: str = DEFAULT_TRACE_ID) -> dict:
+def run_health_probes(self, trace_id: str = DEFAULT_TRACE_ID) -> dict[str, Any]:
     from sqlmodel import Session
 
     from app.core.db import engine
     from app.services.health_probes import run_probes
 
     _set_trace(trace_id)
 
-    def _do() -> dict:
+    def _do() -> dict[str, Any]:
         with Session(engine) as session:
             return run_probes(session=session)
 
     return _run_with_otel_parent(self, _do)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/celery/tasks/job_execution.py` around lines 411 - 423, Update the
return annotations of both `run_health_probes` and its nested `_do` closure from
bare `dict` to `dict[str, Any]`, importing `Any` as needed, so they match the
`run_probes` return contract.

backend/app/api/routes/cron.py (2)

35-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared monitor config values into constants.

checkin_margin, failure_issue_threshold, and recovery_threshold are duplicated between EVALUATION_CRON_MONITOR_CONFIG and the new HEALTH_PROBES_CRON_MONITOR_CONFIG with identical values. As per coding guidelines, repeated literals should be extracted into constants.

♻️ Proposed refactor

+CRON_CHECKIN_MARGIN = 2
+CRON_FAILURE_ISSUE_THRESHOLD = 2
+CRON_RECOVERY_THRESHOLD = 1
+
 EVALUATION_CRON_MONITOR_CONFIG: MonitorConfig = {
     "schedule": {
         "type": "interval",
         "value": settings.CRON_INTERVAL_MINUTES,
         "unit": "minute",
     },
     "timezone": "UTC",
-    "checkin_margin": 2,
+    "checkin_margin": CRON_CHECKIN_MARGIN,
     "max_runtime": 2 * settings.CRON_INTERVAL_MINUTES,
-    "failure_issue_threshold": 2,
-    "recovery_threshold": 1,
+    "failure_issue_threshold": CRON_FAILURE_ISSUE_THRESHOLD,
+    "recovery_threshold": CRON_RECOVERY_THRESHOLD,
 }

 HEALTH_PROBES_CRON_MONITOR_CONFIG: MonitorConfig = {
     "schedule": {
         "type": "interval",
         "value": settings.HEALTH_PROBE_INTERVAL_MINUTES,
         "unit": "minute",
     },
     "timezone": "UTC",
-    "checkin_margin": 2,
+    "checkin_margin": CRON_CHECKIN_MARGIN,
     "max_runtime": 2 * settings.HEALTH_PROBE_INTERVAL_MINUTES,
-    "failure_issue_threshold": 2,
-    "recovery_threshold": 1,
+    "failure_issue_threshold": CRON_FAILURE_ISSUE_THRESHOLD,
+    "recovery_threshold": CRON_RECOVERY_THRESHOLD,
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/api/routes/cron.py` around lines 35 - 47, Extract the duplicated
monitor settings into shared constants and use them in both
EVALUATION_CRON_MONITOR_CONFIG and HEALTH_PROBES_CRON_MONITOR_CONFIG. Replace
the repeated checkin_margin, failure_issue_threshold, and recovery_threshold
literals with the new constants while preserving their current values.

Source: Coding guidelines

---

148-151: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add error handling for consistency with other cron endpoints.

evaluation_cron_job and pending_jobs_cron_job both wrap their logic in try/except, logging the error and capturing it in Sentry before re-raising. health_probes_cron_job has no such guard — if run_health_probes.delay() fails (e.g., broker unavailable), the exception propagates without structured logging or explicit Sentry capture.

🛡️ Proposed error handling

 def health_probes_cron_job() -> dict:
     logger.info("[health_probes_cron_job] Cron job invoked — enqueueing task")
-    async_result = run_health_probes.delay()
-    return {"enqueued": True, "task_id": async_result.id}
+    try:
+        async_result = run_health_probes.delay()
+        return {"enqueued": True, "task_id": async_result.id}
+    except Exception as e:
+        logger.error(
+            f"[health_probes_cron_job] Error enqueueing health probes: {e}",
+            exc_info=True,
+        )
+        sentry_sdk.capture_exception(e)
+        raise

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/api/routes/cron.py` around lines 148 - 151, Wrap the logic in
health_probes_cron_job with a try/except matching evaluation_cron_job and
pending_jobs_cron_job: log the exception with context, capture it in Sentry,
then re-raise it. Keep the existing enqueue and response behavior inside the try
block.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

backend/app/tests/services/test_health_probes.py (1)

21-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add missing -> None return type hints on __init__ methods.

_NonClosingSession.__init__ (line 25) and _FakeProvider.__init__ (lines 41-46) lack return type annotations.

As per coding guidelines, "provide narrow type hints for every function parameter and return value."

✏️ Proposed fix

-    def __init__(self, session: Session):
+    def __init__(self, session: Session) -> None:
         self._session = session

     def __init__(
         self,
         response: Any = None,
         error: str | None = None,
         raise_exc: Exception | None = None,
-    ):
+    ) -> None:

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/tests/services/test_health_probes.py` around lines 21 - 50, Add
the explicit -> None return annotation to the __init__ methods of
_NonClosingSession and _FakeProvider, preserving their existing parameters and
initialization logic.

Source: Coding guidelines

backend/app/services/health_probes.py (1)

160-241: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Previous critical concern resolved — config build now inside try-except in _prepare_probes.

_build_config_and_input is now called inside a dedicated try-except (lines 227-236) rather than unguarded inside _run_probe, so a ValidationError/decode failure on one probe no longer aborts the whole run.

Separately, several probe outcomes use hard-coded string literals ("client_init_failed", "stt_audio_not_configured", "no_response", "not_prepared") that are also asserted against by string equality in the test suite. Consider a small ProbeErrorEnum (or module-level constants) shared between this module and its tests to avoid silent drift between producer and consumer strings.

As per coding guidelines, "Do not use magic values; extract repeated literals into constants, enums, or settings" and "Use an Enum suffix for enum class names."

♻️ Proposed refactor

+class ProbeErrorEnum(str, Enum):
+    CLIENT_INIT_FAILED = "client_init_failed"
+    STT_AUDIO_NOT_CONFIGURED = "stt_audio_not_configured"
+    NO_RESPONSE = "no_response"
+    NOT_PREPARED = "not_prepared"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/services/health_probes.py` around lines 160 - 241, Extract the
repeated probe error literals used by _run_probe and
_prepare_probes—"client_init_failed", "stt_audio_not_configured", "no_response",
and "not_prepared"—into shared module-level constants or a ProbeErrorEnum, using
the required Enum suffix if defining an enum. Replace the producer-side literals
with those shared symbols and update tests to assert against the same symbols
while preserving the existing serialized string values.

Source: Coding guidelines

backend/app/celery/tasks/job_execution.py (1)

248-266: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing type hints on run_prompt_improvement.

Return type and **kwargs are untyped, unlike the sibling run_health_probes task.

As per coding guidelines, "provide narrow type hints for every function parameter and return value; do not use -> Any as a substitute for a specific annotation."

✏️ Proposed fix

-def run_prompt_improvement(self, project_id: int, job_id: str, trace_id: str, **kwargs):
+def run_prompt_improvement(
+    self, project_id: int, job_id: str, trace_id: str, **kwargs: Any
+) -> dict:

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/celery/tasks/job_execution.py` around lines 248 - 266, Update
run_prompt_improvement with narrow type annotations for its **kwargs parameter
and return value, matching the typing approach used by the sibling
run_health_probes task; avoid using Any and preserve the existing task execution
behavior.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes